Repository navigation
Conversation
Roll deferred per-tag buffers at a 256 KiB target without splitting records. Keep completed buffers ordered and hand them to core only from the collector. Drain accepted buffers during graceful shutdown, including after a pause. Detach each in-flight buffer and prevent nested drains when backpressure re-enters the pause callback. Keep ordinary new records subject to memory admission while preserving the callback-owner shutdown allowance. Fixes fluent#12519. Signed-off-by: Michael Renner <terrorobe@github.com>
Exercise chunk bounds, full payload and group context, per-tag order, retained originals, and chained rewrite_tag paths through the real engine. Cover oversized records, partially filled core chunks, backpressure, late shutdown deliveries and new-record admission. The grouped cases require the prerequisite group-framing fix in fluent#12500. Signed-off-by: Michael Renner <terrorobe@github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe emitter now keeps submitted records intact and divides pending records into chunks around a 256 KiB target. It also changes queue draining and shutdown handling. A new runtime test covers burst delivery, chunk sizes, memory pressure, and shutdown scenarios. ChangesEmitter burst handling
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to No confirmed issue currently blocks merging. The inspected shutdown path gives the emitter collector time to drain queued records under normal scheduling. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 18.92% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 37 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Michael Renner <terrorobe@github.com>
This PR targets
masterand builds on the merged group-framing fix. The net change contains only emitter code and runtime tests.Currently, the emitter appends each tag's entire pending buffer to the engine in one piece, so a
rewrite_tagburst can produce chunks far larger than the 2 MB chunk target.Batching changes:
With smaller batches, a backpressure pause can leave accepted records queued in the emitter, and records still queued in a
rewrite_tagemitter at shutdown are currently discarded. Shutdown changes:Fixes #12519.
Testing
Revalidated on Linux/amd64 against
masterat adee1f5, which includes #12500. The focused rewrite-tag, burst, multiline, output-shutdown, and log-event CTest targets passed in plain and ASan/UBSan builds. Rewrite-tag and multiline integration scenarios passed normally and under strict Valgrind; burst and multiline runtime tests also reported no Valgrind errors or leaks. Hosted CI is rerunning on the updated branch.The new runtime test,
tests/runtime/filter_rewrite_tag_burst.c, sends bursts through the real engine and verifies every delivered record: identity, order, payload, timestamp, and group context. It covers:keep=trueduplication, chained rewrites, and native output processorsWithout the emitter changes, the size checks fail. The new test and the existing multiline tests (the other emitter user) pass under ASan/UBSan and Valgrind; the
rewrite_tagand multiline integration tests pass under strict Valgrind.ok-package-testlabel: packaging is unchangedRegression output and verification commands
The automated burst regression uses an in-process collector and library output callbacks. To reproduce the burst manually with a local build, run this
dummy → rewrite_tag → nullpipeline from the repository root:Wait for the output to drain, then press Ctrl-C. With only the prerequisite applied, the whole burst arrives as one chunk:
With this change:
5 × 2,074,968 + 1,976,160 = 12,351,000bytes, matching the baseline.2,048,000 + 262,144 = 2,310,144bytes.Equivalent build and CTest commands, run from the repository root:
To run the existing group-preservation and multiline integration scenarios from the repository root:
Valgrind command, run from the repository root, and output excerpt:
Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit